Make EuiDescribedFormGroup title accessible#2989
Conversation
|
Preview documentation changes for this PR: https://eui.elastic.co/pr_2989/ |
1 similar comment
|
Preview documentation changes for this PR: https://eui.elastic.co/pr_2989/ |
|
But... Now we don't have a To shim the expected functionality, we'd need to add an |
|
In the past, we used to have an issue with having an This solution was making the screen readers read the text twice. I think if we use again I think we should separate things. The title and description of the But I'm open to other solutions. 🙂 |
|
@myasonik Can we not just rely on DOM order and heading structure for screenreaders to read these types of pages. Essentially it renders: |
|
It depends... That structure is really good for things like describing a form. For example, stuff like "This info will only be used for these specific purposes" or "These settings apply to a specific user space and not to others". But it starts to break down when you try describing form fields. For example, not having a legend becomes really difficult is an address "field" (where there actually multiple fields for one "value"). An individual input might be labelled with "Line 1" or "State" and the legend provides the context that this is for a billing address or something. I see how this is tricky to solve with our current setup though so if we want, we can ship this (it's probably better than clobbering semantics like we are right now) but I'd want to open a new issue to reintroduce |
|
I think we should ship this to fix the #2888 bug and we open a new issue to reintroduce the |
cchaos
left a comment
There was a problem hiding this comment.
I agree, let's just fix the a11y issue and then come up with a solution in another issue so we know exactly what the path is forward.
|
Preview documentation changes for this PR: https://eui.elastic.co/pr_2989/ |
thompsongl
left a comment
There was a problem hiding this comment.
LGTM assuming a follow-up issue
Summary
Closes #2888
Previously, it was decided on #2783 to introduce a
<fieldset />to group elements in theEuiDescribedFormGroupcomponent. Because the first element inside the fieldset must be a<legend />it was decided to:<EuiScreenReaderOnly>and add anaria-hidden="true"to the<EuiTitle />This decision created some issues described in #2888. For instance in Kibana Advanced Settings, users could no longer navigate by heading to any setting.
Fix
To fix this issue I decided to:
<fieldset />with a<div role="group" />. Actually, before the change introduced on Described form group a11y #2783 the component was already like that.<EuiTitle />element and we no longer need a<legend />.Checklist
[ ] Check against all themes for compatibility in both light and dark modes[ ] Checked in mobile[ ] Checked in IE11 and Firefox[ ] Added documentation examples